feat: add mobile pairing Settings plugin - #595

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f775fb3a1d

Changes needed: update the imported code-entry authorization, preserve an uncertain transfer outcome, and replace the public QR screenshot; details inline and below.

• [P2] Replace the QR image with portable fixture data. docs/images/pairing/qr.png, also embedded in this public PR description, decodes to an internal deployment address. Disposable identity data does not sanitize the encoded destination. Regenerate it with a generic example relay and update the attachment; do not merely remove visible labels. Both attached images were inspected, and their bytes match the pinned files. No claim is made that the pictured session remains live.
• The existing hidden-window auto-restart finding also remains valid at this head: native cancellation is immediately eligible for renderer renewal. Keep close-triggered cancellation terminal until the pairing view is deliberately reopened.

Star Lord automated source review via Wes’s account. Head 99c0323b02c04d05716e99b22592f6ba78d5191b; base 365061c45b726502a6bdc835a24827c6002a83f0. Source and publication-material inspection only; no tests, app launches, pairing sessions, or security probes. Updated-mobile/native end-to-end behavior remains unverified. This is a non-blocking COMMENT review, not approval.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72ec901c16

Changes still needed at this head; I revalidated these existing findings rather than opening duplicate inline threads:

• P2 — Preserve uncertainty after post-send transport failure. The new payload_sent flag only affects expire; Ok(Err(message)) still becomes an ordinary error. Route ambiguous post-publication I/O failures to Check your phone, retaining explicit phone import rejection as an error, and cover both outcomes. Existing discussion.
• P2 — Keep window-close cancellation terminal. The macOS hide path still leaves the component mounted, and cancelled still triggers automatic renewal. Existing discussion and repair.
• P2 — Replace the public QR fixture. Both PR attachments match the pinned files; the QR image is byte-identical to the previously reviewed image, so the encoded internal deployment destination remains. Regenerate it with a portable fixture relay. Prior finding.

The independent desktop-code/five-attempt guard and timeout-specific uncertainty path address the corresponding earlier defects in source.

Star Lord’s automated source review via Wes’s account (wesbillman). Head: 72ec901c162dbebff3027ec6d1e130772cf3e18c; base: c8e7abb0eeeb7c5231a2c767edc28bef86c2a4ef. No code, tests or app executed. Native/updated-mobile interoperability and success/error/retry focus remain unverified; the one CI snapshot still had browser jobs running.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7cc00375e2

Changes still needed in the existing follow-ups; no duplicate inline threads:

• P2 — Legacy denial: provide a mismatch action that sends user_denied before disposing the session; “Codes match” remains the only action.
• P2 — Import acknowledgement: coordinate the mobile fix so success follows durable import and the generation check. Rechecked block/buzz@1e67395fd19a8b4de0cecca97cd1b36842fe8b77: pairing_provider.dart:750 still sends success before persistence at :763–764, allowing false desktop success.
• P3 — Connection timeout: bound connection establishment and surface Try again; a stalled connection still becomes expiry and automatically renews.

The post-publication uncertainty and terminal-cancellation fixes address my previous findings in source. Both inspected attachments match the pinned files, and the QR now encodes the portable example relay.

Star Lord’s automated source review via Wes’s account (wesbillman). Head 7cc00375e2bb3fc4bdc15b654bde496c067f2253; base c8e7abb0eeeb7c5231a2c767edc28bef86c2a4ef. No tests or app executed; live pairing, macOS hide/reopen and keyboard-focus transitions remain unverified. The single CI snapshot was pending. Non-blocking COMMENT, not approval.

One existing change remains; no duplicate inline thread:

• P2 — Import acknowledgement: the mobile repair is explicitly deferred, but desktop still says “Phone paired” / “Your account is ready,” and docs/pairing.md:34–36 implies updating the phone supplies the guarantee. Current mobile source still acknowledges before persistence; an import failure can therefore follow desktop success. Keeping the mobile repair separate is fine: until it ships, make desktop success/docs describe the transfer and ask users to verify the account on their phone, rather than guarantee durable import.

Legacy denial and bounded connection/setup timeouts now address the other two follow-ups in source.

Star Lord’s automated source review via Wes’s account (wesbillman). Head 8a583c54f607d0154357ee35e81c5aa88eb335c1; base 97155e7cbd1f97ecedfab5ec8f36310f6caf2e89. Source-only; no tests/app execution. Live updated-mobile pairing and native/keyboard acceptance remain unverified. One CI snapshot: JS/DCO passed; Rust/browser journeys running; Windows skipped. Non-blocking COMMENT, not approval.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

One remaining P2; changes recommended before merge. Submitted as COMMENT under this repository’s agent-review policy, not approval. Native integration, protocol/security, and renderer review lanes are complete; the actionable finding is inline.

Reviewed head 8a583c54f607d0154357ee35e81c5aa88eb335c1 against target 97155e7cbd1f97ecedfab5ec8f36310f6caf2e89 (merge base d27bed1cb354af03ddb796e17de6c0a8053b9d07).

• Validation: independently reproduced the cancellation ordering using the pinned production client with inert import stubs/fake native state; native outcome loss is source-derived. Verified hosted logs at merge 9391f58118f1ed838349c8759b778de60eabddc5: 7,984 JS tests, 80 protocol tests + 2 doctests, and the native package’s 295 tests passed (7 ignored). All seven Chromium pairing journeys passed. Browser coverage uses fixture IPC, not real handoff.
• Earlier fixes: independent desktop-code authorization/guess budget, legacy denial, bounded connection/setup, shared visible-QR deadline, terminal cancellation, and post-send transport/timeout uncertainty are present. Both PR images match pinned blobs; locally decoded QR destination is the reserved example relay.
• Remaining gates/limits: repair the inline cancellation defect with native-owner and client/UI regressions, and clear remaining CI. No live phone/Keychain or macOS hide/reopen run by this review. Author-reported successful live code-entry testing is useful acceptance evidence, but the separately scoped mobile import-ack repair remains unshipped per the existing discussion; desktop success therefore does not establish durable import on affected phones. The optional security-documentation note is not a request to redesign the agreed phone-entry UX.

No further changes requested on this follow-up. Native cancellation now preserves and returns the possible-transfer outcome, and the desktop copy/docs ask users to verify sign-in on the phone rather than guarantee durable import; the screen-observer limitation is also documented.

Star Lord’s automated source review via Wes’s account (wesbillman): head 0b1f159dbb24adfd12d3f9ca5f7e5a8dd85f6b19, base 97155e7cbd1f97ecedfab5ec8f36310f6caf2e89. Source-only—no tests or app execution; hosted CI required passed in one snapshot, Windows was skipped, and live phone/native-window/keyboard-focus behavior remains unverified by this review. This COMMENT is not approval.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

One P2 remains; hold approval until the inline relay-lifetime defect is addressed. All three delegated lanes are complete and reconciled with native lifecycle/integration review. The previous cancellation finding is repaired. The other two inline notes are optional.

Reviewed head 156d5d87d9decd9e0544b4495c582c67cf10251b against base/merge-base 484a793261a070ff27e4683969a40fb6ccf2dbe5. Pairing source/tests are byte-identical to reviewed 0b1f159d; incoming integration changes were separately inspected.

Current-head CI is green (21 successful checks, Windows native validation skipped). Eight inert production-client ordering probes passed at 0b1f159d. The new finding is independently source-verified, not an executed sidecar reproduction; existing browser journeys substitute IPC.

Merge criteria: reconcile the supported relay’s lifetime with QR renewal and cover that boundary, retaining terminal cancellation and post-publication uncertainty. No independent live phone, native hide/reopen, or assistive-technology acceptance was performed. Earlier live code-entry success is author-reported; the separately scoped mobile durable-import fix remains unshipped, so desktop acknowledgement still does not certify durable phone login.

Changes needed: refresh the integration lockfile; the current-base merged build fails before compiling the native browser fixture (P2 inline). The pre-connect deadline addresses the prior relay-lifetime finding while preserving uncertain-transfer outcomes; both screenshots match the previously inspected portable fixtures.

Star Lord’s automated source review via Wes’s account. Head e73ea9148f7ae978c54bd8d3ef48b0f97eb56322; base 936279f300d97ba2fb7860cb0eaab7bbd2ac91d0. Source-only: no tests or app execution. Browser journeys were skipped after the fixture failure; Rust was pending and Windows skipped. Live-phone, native-window and keyboard-focus acceptance remain unverified. Non-blocking COMMENT, not approval.

No further actionable changes requested on this follow-up: the lockfile now disambiguates the plugin manager’s nostr 0.45.5 dependency, and the merged-tree locked fixture build succeeds.

Star Lord automated source review via Wes’s account; head 01d9320e3f409dff8a27b4d009df1b2d5388c835, base 5feaddbfcbc41f93c259610492978eb886c9055c; COMMENT only, not approval.

No local tests or app execution; other CI lanes were pending in the inspected snapshot, Windows was skipped, and native/phone pairing and keyboard-focus acceptance remain unverified.